Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add t0123 & t0113 - gateway - json - cbor - symlinks #25

Merged
merged 23 commits into from
Apr 4, 2023
Merged

feat: add t0123 & t0113 - gateway - json - cbor - symlinks #25

merged 23 commits into from
Apr 4, 2023

Conversation

laurentsenta
Copy link
Contributor

@laurentsenta laurentsenta commented Mar 27, 2023 •
edited
Loading

Contributes to #26

Tasks:

  • 123: test_dag_pb_conversion
    • Test structure
    • implement dag node getter
    • Implement the body checks
  • 123: test_plain_codec
    • Test structure
    • Implement the fixture codec for json
  • 123: Pathing, traversal over DAG-JSON and DAG-CBOR
  • refactor the fixture into a more usable apis.

Notes

bytes equalities
the current test compares kubo and gateway outputs bytes to bytes, for example:

ipfs dag get --output-codec dag-$format $DIR_CID > expected
test_cmp expected my_gateway_output

For simple cases, we replace with a higher level comparator (JSON uses reflect.DeepEqual),
We stick to a bytes-to-bytes comparison with CBOR for now.

@laurentsenta laurentsenta changed the title feat: addt0123 - gateway - json - cbor feat: add t0123 - gateway - json - cbor Mar 27, 2023
@laurentsenta laurentsenta mentioned this pull request Mar 27, 2023
Closed
20 tasks
@laurentsenta laurentsenta marked this pull request as draft March 27, 2023 14:59
@laurentsenta laurentsenta changed the title feat: add t0123 - gateway - json - cbor feat: add t0123 & t0122 - gateway - json - cbor - tar Mar 29, 2023
@laurentsenta
Copy link
Contributor Author

Using shell check, this is what happens on error:

CleanShot 2023-03-30 at 11 01 42@2x

@laurentsenta laurentsenta changed the title feat: add t0123 & t0122 - gateway - json - cbor - tar feat: add t0123 & t0122 & t0113 - gateway - json - cbor - tar - symlinks Mar 30, 2023
@laurentsenta laurentsenta changed the title feat: add t0123 & t0122 & t0113 - gateway - json - cbor - tar - symlinks feat: add t0123 & t0113 - gateway - json - cbor - symlinks Mar 31, 2023
@laurentsenta laurentsenta mentioned this pull request Mar 31, 2023
Merged
@laurentsenta laurentsenta marked this pull request as ready for review March 31, 2023 10:17
galargh
galargh reviewed Mar 31, 2023
View reviewed changes
fixtures/t0123/dag-pb.json Outdated Show resolved Hide resolved
galargh
galargh approved these changes Mar 31, 2023
View reviewed changes
Copy link
Contributor

@galargh galargh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏆

tests/t0123_gateway_json_cbor_test.go Show resolved Hide resolved
tests/t0123_gateway_json_cbor_test.go Show resolved Hide resolved
tooling/car/unixfs.go Outdated Show resolved Hide resolved
tooling/check/check.go Show resolved Hide resolved
laurentsenta
laurentsenta commented Apr 3, 2023
View reviewed changes
tests/t0123_gateway_json_cbor_test.go
test_cmp expected plain_output
'
*/
// TODO(lidel): Note we disable this test, we check the payloads above.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lidel a note here, we ignore a few tests: they compare two requests with each other.
Instead, we test each request.

@laurentsenta laurentsenta mentioned this pull request Apr 4, 2023
Open
2 tasks
@laurentsenta laurentsenta merged commit 023888a into main Apr 4, 2023
@laurentsenta laurentsenta deleted the feat/add-t0123 branch April 4, 2023 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@galargh galargh galargh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@laurentsenta @galargh